Shared textures OpenGL #548
Shared textures OpenGL #548ujpv wants to merge 1 commit intokharitonov/shared_texture_refactor_metalfrom
Conversation
ujpv
force-pushed
the
kharitonov/shared_texture_opengl
branch
2 times, most recently
from
f9181fe to
d218421
Compare
ujpv
force-pushed
the
kharitonov/shared_texture_opengl
branch
3 times, most recently
from
c29910a to
9be77e8
Compare
| (JNIEnv *env, jclass clazz, jlong texture) { | ||
| if (gTextureType == OPENGL_TEXTURE_TYPE) { | ||
| GLuint texId = (GLuint)texture; | ||
| glDeleteTextures(1, &texId);\ |
| final static TexturePixelLayout RGBA = new TexturePixelLayout(0, 1, 2, 3); | ||
| final static TexturePixelLayout BGRA = new TexturePixelLayout(2, 1, 0, 3); | ||
| static TexturePixelLayout get(int textureType) { | ||
| return switch (textureType) { | ||
| case SharedTextures.OPENGL_TEXTURE_TYPE -> TexturePixelLayout.RGBA; | ||
| case SharedTextures.METAL_TEXTURE_TYPE -> TexturePixelLayout.BGRA; |
There was a problem hiding this comment.
We seem to expect different pixel layouts for Metal and OGL, however for *TextureWrapperSurfaceData classes we always use ColorModel.getRGBdefault(), which seems inconsistent - could you check that?
There was a problem hiding this comment.
I made it consistent with how it's done for VolatileImage. The color model is taken now from the GraphicsConfig.
However, look like those color model from TextureWrapperSurfaceData is the color model on the public image API side. It doesn't have to be the same as the in the accelerated surface.
ujpv
force-pushed
the
kharitonov/shared_texture_opengl
branch
2 times, most recently
from
de0bfbd to
60ce7f2
Compare
ujpv
changed the base branch from
kharitonov/main
to
kharitonov/shaared_textures_refactor_mac
ujpv
changed the base branch from
kharitonov/shaared_textures_refactor_mac
to
kharitonov/shared_texture_refactor_metal
ujpv
force-pushed
the
kharitonov/shared_texture_opengl
branch
from
60ce7f2 to
98362d9
Compare
YaaZ
left a comment
YaaZ
left a comment
There was a problem hiding this comment.
LGTM overall. Although I see a lot of instanceofs, I guess you're intentionally trying to keep your code separate from the JDK's, right?
| -framework Cocoa -framework SystemConfiguration | ||
| BUILD_JDK_JTREG_LIBRARIES_LIBS_libSharedTexturesTest := \ | ||
| -framework Cocoa -framework Metal | ||
| -framework Cocoa -framework Metal -framework OpenGL |
There was a problem hiding this comment.
I forgot whether I already asked this, but do we really need an OGL implementation on macOS, given that it's deprecated there?
There was a problem hiding this comment.
Yes, we talked about it and concluded that it won't heart. :)
We still have this pipeline, the OS still supports it so far.
| private final Image image; | ||
|
|
||
| public WGLTextureWrapperSurfaceData(WGLGraphicsConfig gc, Image image, long textureId) { | ||
| super(null, gc, ColorModel.getRGBdefault(), RT_TEXTURE); |
There was a problem hiding this comment.
In other places you take color model from the GC, this one looks inconsistent with the others
Yes, sort of. |
2 participants
The corresponding JBR API review:
JetBrains/JetBrainsRuntimeApi#20